Skip to content

Conversation

@kxbnb
Copy link

@kxbnb kxbnb commented Jan 23, 2026

Summary

Fixes #1717

Adds handling for EndOfStream and ClosedResourceError in send_request(). These exceptions can be raised by response_stream_reader.receive() but weren't being caught, leading to UnboundLocalError at the subsequent isinstance check.

Changes

  • Added except (anyio.EndOfStream, anyio.ClosedResourceError) block after the existing TimeoutError handler
  • Converts these exceptions to McpError with CONNECTION_CLOSED error code

Validation

  • uv run pyright passes with 0 errors
  • All 758 tests pass

Testing

tests/shared/test_session.py::test_connection_closed PASSED

Closes #1717

@maxisbey maxisbey added bug Something isn't working needs repro needs additional information to be able to reproduce bug labels Feb 10, 2026
@maxisbey
Copy link
Contributor

Thanks for the fix! Before we merge, could you:

  1. Share a reproduction scenario that triggers this in practice
  2. Add a test that fails on main but passes with your fix

We want to make sure this is a real-world issue, not just a theoretical code path.

AI Disclaimer

Fixes modelcontextprotocol#1717

The `send_request` method only catches `TimeoutError` from the
`response_stream_reader.receive()` call. If `receive()` raises
`EndOfStream` or `ClosedResourceError` (e.g., when the connection
closes unexpectedly), these exceptions propagate without being
caught, potentially leaving `response_or_error` unassigned and
causing an `UnboundLocalError` at the subsequent isinstance check.

This adds explicit handling for these stream closure exceptions,
converting them to `McpError` with `CONNECTION_CLOSED` error code.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@kxbnb kxbnb force-pushed the fix/unbound-local-error-send-request branch from a3f2133 to 0356494 Compare February 10, 2026 16:41
@kxbnb
Copy link
Author

kxbnb commented Feb 10, 2026

Rebased on upstream/main and added a regression test.

Reproduction: The race happens when the per-request response stream's send side is closed (e.g. receive loop's finally block calls aclose()) before send_request reads from it. response_stream_reader.receive() raises EndOfStream, which wasn't caught, so response_or_error is never assigned and the isinstance check on the next line hits UnboundLocalError.

In practice this shows up when the server disconnects abruptly and the receive loop's finally block and send_request race on the response stream — the loop closes the stream before or without sending the error object.

Test: test_response_stream_closed_raises_mcp_error reproduces this by closing the response stream's send side directly while the server connection stays open (so the receive loop never enters its finally block). Fails on main with unhandled EndOfStream, passes with the fix.

…xtprotocol#1717)

Adds a test that reproduces the race condition where the per-request
response stream is closed before send_request reads from it. Without
the fix, EndOfStream propagates unhandled. With the fix, it's converted
to MCPError with CONNECTION_CLOSED.

The test closes the response stream's send side directly while the
server connection stays open, bypassing the receive loop's graceful
error injection.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@kxbnb kxbnb force-pushed the fix/unbound-local-error-send-request branch from 0356494 to 480c6d6 Compare February 10, 2026 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working needs repro needs additional information to be able to reproduce bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UnboundLocalError in send_request when response_or_error is not assigned

2 participants